Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate rendering templates with . in the name #38858

Merged
merged 1 commit into from
Apr 8, 2020
Merged

Deprecate rendering templates with . in the name #38858

merged 1 commit into from
Apr 8, 2020

Conversation

jhawthorn
Copy link
Member

Allowing templates with . introduces some ambiguity. Is index.html.erb a template named index with format html, or is it a template named index.html without a format? We (humans) know it's probably the former, but if we asked ActionView to render index.html we would currently get some combination of the two: a Template with index.html as the name and virtual path, but with html as the format.

This deprecates having "." anywhere in the template's name, we should reserve this character for specifying formats. I think in 99% of cases this will be people specifying index.html instead of simply index.

This was actually once deprecated in the 3.x series (removed in 6c57177) but I don't think we can rely on nobody having introduced this in the past 8 years 馃槄.

@jhawthorn
Deprecate rendering templates with . in the name
f84773a 
Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.

This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.

This was actually once deprecated in the 3.x series (removed in
6c57177) but I don't think we can rely
on nobody having introduced this in the past 8 years.
@jhawthorn jhawthorn marked this pull request as ready for review April 1, 2020 20:23
@jhawthorn jhawthorn merged commit 32b059b into rails:master Apr 8, 2020
@kaspth
Copy link
Contributor

kaspth commented Apr 14, 2020

Looks like this deprecation message also fires on dynamic renders.

We've got a case of:

<%= render "postings/rows/#{posting.kind}", posting: posting %>

triggering:

DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: postings/rows/_#{posting.kind} (called from fetch at /Users/kaspth/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/concurrent-ruby-1.1.6/lib/concurrent-ruby/concurrent/map.rb:172)

@dhh
Copy link
Member

dhh commented Apr 21, 2020

Going to revert this for now, because the deprecation warnings is driving me batty.

@dhh dhh mentioned this pull request Apr 21, 2020
Merged
kamipo added a commit that referenced this pull request Apr 27, 2020
@kamipo
Revert "Update actionview CHANGELOG"
977ba0e 
This reverts commit 960ceb2.

Ref #39012, #38858.

[ci skip]
@jhawthorn jhawthorn mentioned this pull request Apr 27, 2020
Merged
@corytheboyd-cirrusmd
Copy link

corytheboyd-cirrusmd commented Jun 30, 2021 •
edited

Is this revert going into a release anytime soon? Getting tons of these deprecation warnings on 6.1.4 still

@jhawthorn
Copy link
Member Author

@corytheboyd-cirrusmd Yes, this deprecation shipped in Rails 6.1. In Rails 7.0 attempting to use a . in a template name will result in a MissingTemplate exception.

In most cases this deprecation warning occurs when trying to render a template like render "index.html". Instead we should be using render "index" (or render "index", formats: :html if the format is necessary). If you're seeing this deprecation anywhere you don't think you should be please file a new issue 馃檪.

@corytheboyd-cirrusmd
Copy link

corytheboyd-cirrusmd commented Jun 30, 2021 •
edited

@corytheboyd-cirrusmd Yes, this deprecation shipped in Rails 6.1. In Rails 7.0 attempting to use a . in a template name will result in a MissingTemplate exception.

In most cases this deprecation warning occurs when trying to render a template like render "index.html". Instead we should be using render "index" (or render "index", formats: :html if the format is necessary). If you're seeing this deprecation anywhere you don't think you should be please file a new issue 馃檪.

@jhawthorn Gotcha, thanks for the quick reply! I was shooting from the hip with that comment amidst a pretty large Rails 6.0 => 6.1 upgrade hah, will actually dig in now and look for those offenses. Cheers 馃嵒 !

shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this pull request Aug 4, 2021
@shaun-technovation
Update "render partial" calls
cf2b221 
This change is to address these deprecation warnings, more details in
this thread/post:
rails/rails#38858 (comment)

Refs: #2872
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this pull request Aug 5, 2021
@shaun-technovation
Update "render partial" calls
f14ad5f 
This change is to address these deprecation warnings, more details in
this thread/post:
rails/rails#38858 (comment)

Refs: #2872
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this pull request Aug 5, 2021
@shaun-technovation
Update "render partial" calls
47f2a80 
This change is to address these deprecation warnings, more details in
this thread/post:
rails/rails#38858 (comment)

Refs: #2872
shaun-technovation added a commit to Iridescent-CM/technovation-app that referenced this pull request Aug 5, 2021
@shaun-technovation
Update "render partial" calls
d5a3bea 
This change is to address these deprecation warnings, more details in
this thread/post:
rails/rails#38858 (comment)

Refs: #2872
@liaham liaham mentioned this pull request Mar 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaspth kaspth kaspth approved these changes

@eileencodes eileencodes eileencodes approved these changes

Assignees
No one assigned
Labels
actionpack actionview
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jhawthorn @kaspth @dhh @corytheboyd-cirrusmd @eileencodes